Skip to content

fix(mobile): Reconnect and Retry now on a switched-off environment switch it back on - #28628

Open
only21mil wants to merge 6 commits into
pingdotgg:mainfrom
only21mil:fix/audit-m047b
Open

only21mil wants to merge 6 commits into
pingdotgg:mainfrom
only21mil:fix/audit-m047b

Conversation

@only21mil

@only21mil only21mil commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On mobile, an environment switched off in Settings › Environments keeps a way to reconnect on screen: the "<name> is not connected" pill on a thread, and Retry now on the terminal, review and file preview notices. Tapping it does nothing, because both call retryNow and the supervisor ignores retries while the environment is switched off; the only way out is the switch in Settings.

Change

A new useRetryEnvironment hook switches a switched-off environment back on with setEnabled, and otherwise calls retryNow as before. Every mobile reconnect control now goes through it: the shared onReconnectEnvironment handler (thread pill, new-task Reconnect, Settings › Environments, Connections) and the terminal, review sheet and file preview. useConnectionController.retryEnvironment had no other caller and is removed.

Scope

A bug fix for visible actions that do nothing. It is a very small, focused fix for an obvious bug, so it needs no issue; it changes the mobile app only and makes the same decision as #24691 on web. It also covers the pill fix from #28408, which I closed in favour of this PR so one PR fixes the whole bug.

Verification

main d86ee1e (it includes #34207) and this branch at 339d070 (it merges that main), on an iPhone simulator (iOS 26.5) and an Android 16 emulator (arm64, software rendering), against a test server named "Test machine", with taps through the T3 Device panel.

For each control: switch Test machine off in Settings › Environments, open the screen by its link, tap the control, then check Settings › Environments. On main the pill or notice stays and the switch stays off. On the branch the screen loads, and the switch is back on and connected. The new hook test passes on the branch and fails with main's behavior (details).

After the tap, iOS:

main d86ee1e branch 339d070
iOS, pill iOS, main d86ee1e: pill stays
iOS, main d86ee1e261: pill stays
iOS, branch 339d070: thread loaded
iOS, branch 339d0700d1: thread loaded
iOS, terminal iOS, main d86ee1e: still disconnected
iOS, main d86ee1e261: still disconnected
iOS, branch 339d070: terminal loaded
iOS, branch 339d0700d1: terminal loaded
iOS, review iOS, main d86ee1e: still disconnected
iOS, main d86ee1e261: still disconnected
iOS, branch 339d070: diff loaded
iOS, branch 339d0700d1: diff loaded
iOS, file preview iOS, main d86ee1e: still disconnected
iOS, main d86ee1e261: still disconnected
iOS, branch 339d070: image loaded
iOS, branch 339d0700d1: image loaded

Not checked: an environment that can't be switched on (unsupported, no saved route). I read that path in the code but did not run it.

Android, the merge with #34207, method, before the tap, and tests

After the tap, Android:

main d86ee1e branch 339d070
Android, pill Android, main d86ee1e: pill stays
Android, main d86ee1e261: pill stays
Android, branch 339d070: thread loaded
Android, branch 339d0700d1: thread loaded
Android, terminal Android, main d86ee1e: still disconnected
Android, main d86ee1e261: still disconnected
Android, branch 339d070: terminal loaded
Android, branch 339d0700d1: terminal loaded
Android, review Android, main d86ee1e: still disconnected
Android, main d86ee1e261: still disconnected
Android, branch 339d070: diff loaded
Android, branch 339d0700d1: diff loaded
Android, file preview Android, main d86ee1e: still disconnected
Android, main d86ee1e261: still disconnected
Android, branch 339d070: image loaded
Android, branch 339d0700d1: image loaded

The branch merges main d86ee1e. The one hand-resolved part is the import block of ReviewSheet.tsx, where #34207 (Android sheets scroll without per-feature platform checks) added SheetPane and dropped cn; the merge keeps #34207's import and its restyled sheet, and drops environmentCatalog, which this branch no longer uses there. Against the live head nothing this PR does changes. On the Android emulator the Review changes screen scrolls the same on main and on the branch: one swipe up on a 124-line diff brings Note line 077 near the top on both (recording on main, recording on the branch). Both show the same development error toast on that screen ("Maximum update depth exceeded", raised from main's ReviewSheet.tsx line 585), so it does not come from this change.

Android, Review changes top after one swipe up
main d86ee1e Android, main d86ee1e: Review at the top
Android, main d86ee1e261: Review at the top
Android, main d86ee1e: Review after one swipe, Note line 077 near the top
Android, main d86ee1e261: Review after one swipe, Note line 077 near the top
branch 339d070 Android, branch 339d070: Review at the top
Android, branch 339d0700d1: Review at the top
Android, branch 339d070: Review after one swipe, Note line 077 near the top
Android, branch 339d0700d1: Review after one swipe, Note line 077 near the top

Problem and change in full.

On mobile, an environment switched off in Settings › Environments keeps its screens open with a way to reconnect: the floating "<name> is not connected" pill on a thread, and the Retry now button on the terminal, review and file preview connection notices. Both call retryNow, which does nothing while the environment is switched off, because the supervisor ignores retries until the environment is wanted again. The pill and the notice stay, and the only way out is the switch in Settings.

#24691 fixed the same dead button on web and left the mobile notices for later.

A new useRetryEnvironment hook in features/connection reads the environment's catalog entry. If the environment is switched off (enabled === false), it switches it back on with setEnabled. Otherwise it calls retryNow as before. Every mobile reconnect control now goes through it:

  • useRemoteConnections().onReconnectEnvironment, the shared handler behind the thread pill, the new-task Reconnect, Settings › Environments and Connections.
  • The terminal, review sheet and file preview, in place of their direct retryNow command. The file preview still reloads after the tap, as before.

useConnectionController.retryEnvironment had no other caller and is removed, so the app has one path to retryNow from a user action. An environment that can't be switched on (unsupported, with no saved route) fails at once with the existing "environment toggle" error, the same as the Settings switch.

Method. Taps went through the T3 Device panel; the Android pill has no accessibility label, so it was tapped at its position. On main and the branch, the app was relaunched before the review step, so no diff was cached.

Both revisions ran the dev client built for main's native fingerprint, which the merge shares, with JavaScript served by Metro from each revision's own checkout, against one test server from the main checkout (the branch changes only the mobile app). The server held a sample project (one uncommitted edit, one image) and finished Replay (dev) threads.

For the file preview, the image is opened once while connected, then the environment is switched off and the preview opened again: on today's main a file screen opened with no session shows "Files unavailable" before it reaches the preview. The switch state after each tap was read in Settings › Environments (off on main, on with the merge, every control, both platforms).

Before the tap on the branch (on main the screens look the same before and after):

iOS Android
pill iOS, branch 339d070: pill before the tap
iOS, branch 339d0700d1: pill before the tap
Android, branch 339d070: pill before the tap
Android, branch 339d0700d1: pill before the tap
terminal iOS, branch 339d070: notice before Retry now
iOS, branch 339d0700d1: notice before Retry now
Android, branch 339d070: notice before Retry now
Android, branch 339d0700d1: notice before Retry now
review iOS, branch 339d070: notice before Retry now
iOS, branch 339d0700d1: notice before Retry now
Android, branch 339d070: notice before Retry now
Android, branch 339d0700d1: notice before Retry now
file preview iOS, branch 339d070: notice before Retry now
iOS, branch 339d0700d1: notice before Retry now
Android, branch 339d070: notice before Retry now
Android, branch 339d0700d1: notice before Retry now

The hook test on main d86ee1e with only the missing hook added as a stub that calls retryNow, as main's controls do:

 FAIL  |react| src/features/connection/useRetryEnvironment.test.tsx > useRetryEnvironment > Retry now with the environment enabled: false
AssertionError: expected { enabled: false, retries: 1 } to deeply equal { enabled: true, retries: +0 }
      Tests  1 failed | 1 passed (2)

On this branch (339d070):

 ✓ |react| src/features/connection/useRetryEnvironment.test.tsx > useRetryEnvironment > Retry now with the environment enabled: false
 ✓ |react| src/features/connection/useRetryEnvironment.test.tsx > useRetryEnvironment > Retry now with the environment enabled: true

Earlier runs on main 7462f23 (head 0e8284b, merge 3e094786db; iPhone 17 Pro simulator, iOS 26.5, and an Android 15 emulator, Pixel 7 profile, arm64, SwiftShader software rendering) and on main f419698 (merge ca212c1) showed the same. main has since changed ReviewSheet.tsx (#34207), so those captures are not reused.

Prior PRs

#24691 by Jacksondr5 fixed this on web and left these mobile notices for later. #28408, also mine, carried the pill half and is folded into this PR.

#5599 and then #5598 (DominicVonk) included this same action: tap the red composer status to switch a disabled environment on. #5599 closed in favour of #5598, which closed for its much broader availability model. The enabled flag and commands this fix uses landed later in #11478.

Generated with Claude Opus 5.5 and GPT-6.1 Terra in T3 Code Nightly

On mobile, the "Retry now" button in the connection notice on the terminal,
review and file preview screens did nothing for an environment that was
switched off: the connection supervisor ignores retries while the
environment is disabled.

A new useRetryEnvironment hook switches a disabled environment back on
instead of retrying it, matching the web fix in pingdotgg#24691, and the three
screens use it. A hook test covers both cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched Author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7fc31ce

Macroscope's review found this PR approvable — Mobile “Retry now” now re-enables a switched-off environment using the existing connection toggle. The change is limited to explicit retry actions and leaves product defaults unchanged.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The mobile app adds a shared environment retry hook. It enables an environment when its catalog entry is explicitly disabled; otherwise, it retries the environment. File preview, review, terminal, and remote reconnect flows use the hook.

Changes: Mobile environment retry handling

Layer / File(s) Summary
Retry behavior and tests
apps/mobile/src/features/connection/useRetryEnvironment.ts, apps/mobile/src/features/connection/useRetryEnvironment.test.tsx, apps/mobile/vite.config.ts
The hook enables an environment when its catalog entry is disabled and otherwise retries it. Parameterized tests cover initially disabled and enabled environments. The test is included in the React test project.
Adopt retry behavior in mobile flows
apps/mobile/src/features/files/WorkspaceFilePreviewError.tsx, apps/mobile/src/features/review/ReviewSheet.tsx, apps/mobile/src/features/terminal/ThreadTerminalRouteScreen.tsx, apps/mobile/src/state/use-remote-environment-registry.ts, apps/mobile/src/features/connection/useConnectionController.ts
File preview, review, terminal, and remote reconnect flows use useRetryEnvironment. The connection controller no longer exposes its retry command.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: Minimal · up to 339d0

This change makes Reconnect and Retry turn a switched-off environment back on instead of doing nothing. No actionable merge-blocking risk was found in the reviewed change.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title names the main change: a switched-off mobile environment is switched back on by the Reconnect and Retry actions. The changeset adds useRetryEnvironment, which calls setEnabled when the envir…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description follows the repository template closely. It has a Problem section that states the bug and the reproduction steps. It has a Change section that explains the hook and every call site. It…
Full details: Title check

Explanation

The title names the main change: a switched-off mobile environment is switched back on by the Reconnect and Retry actions. The changeset adds useRetryEnvironment, which calls setEnabled when the environment has enabled === false and otherwise calls retryNow. The title is concise, uses the conventional commit form, and matches the changed files and the PR objectives.

Full details: Description check

Explanation

The description follows the repository template closely. It has a Problem section that states the bug and the reproduction steps. It has a Change section that explains the hook and every call site. It has a Scope section that explains why no issue is linked, names the related PRs, and says the change is mobile-only. It has a Verification section with the platforms, the commands run, the observed before and after results, the hook test output, and a statement of what was not checked. The description is long, but the required information is present. The one gap is that the Scope section does not link an issue. The author explains why no issue is needed, which the template allows for a small fix of an obvious bug.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between ca212c1 and 339d070.

Files selected for processing: 1

  • apps/mobile/src/features/review/ReviewSheet.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

maria-rcks and others added 3 commits November 6, 2026 10:56
The floating "not connected" pill and the other Reconnect controls called
retryNow, which does nothing while the environment is switched off. They
now use useRetryEnvironment, the same helper as Retry now on the terminal,
review and file preview, so they switch the environment back on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Reconnect pill now calls useRetryEnvironment directly, so its
switched-off case is the one useRetryEnvironment.test.tsx already
covers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@only21mil only21mil changed the title fix(mobile): Retry now on a switched-off environment switches it back on fix(mobile): Reconnect and Retry now on a switched-off environment switch it back on Nov 10, 2026
@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

I merged main into this branch because its new commits changed files this PR touches. The merge had no conflicts, and nothing else changed.

Resolved the import block of apps/mobile/src/features/review/ReviewSheet.tsx by hand: kept main's SheetPane import from pingdotgg#34207 and dropped both the cn import that pingdotgg#34207 removed and the environmentCatalog import that this branch's useRetryEnvironment change left unused.
@only21mil

Copy link
Copy Markdown
Contributor Author

Note

🤖 Opus 5.5 replying on behalf of Victor.

I merged main to clear the conflict with #34207.
The one part I resolved by hand is the import block of ReviewSheet.tsx: main's import is kept, and two imports nothing uses any more are dropped. Nothing else changed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched Author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants